-
-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
🧹Components
: Light syntactical sugar for finding Components
#1212
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this is fine?
Personally, it feels like component(:card...)
adds another layer of non-trivial indirection to the alternative, which would be CardComponent
, making it easier for someone that's reading the code to see which class is being used and find the relevant code.
Could it be that it is the ...Component
bit in the naming that feels too heavyweight? Could that be improved by adopting a different naming convention for the Component classes that doesn't include Component
in the name? This might be a terrible idea, too, opening us to potential confusion with models/controllers. 🤷🏼♀️
Your patient, doctor.
- #1187 This is probably a premature optimization, but I hate writing the entire class name out in template files! I don't know why! It just bothers me!
b350a7c
to
b8e92f0
Compare
Yea, I think it's a pretty tiny nub of indirection at the moment; but I am also hoping for a world where I can do something like |
@zspencer how are you feeling about this PR? Ok to close it, or do you want to merge it? |
I have deprioritized it since you didn't seem keen and I don't have a compelling use-case yet. |
Now that I've spent some more time with ViewComponents, I think I see where you were coming from here: having to do The thing that I'm less sure about is replacing entire class names with a symbol, because I worry that it will make it more confusing to try to find a component if the reader of the code doesn't know about this helper. But maybe that's overly cautious of me, folks can search for the We could go with what you have here, plus shoving the <%= component :button , variant: :foo %> (if we go down this route, maybe it should also be able to take a string, so that we can deal more easily with nested classes, e.g. "marketplace/tax_rate" -> Marketplace::TaxRateComponent) Or we could go the more-typing-but-maybe-clearer version where we make a class method <%= ButtonComponent.render(variant: :foo) %> Thoughts? |
I've been exploring pulling out little factory helper methods into the Components themselves. This gives us a clear linkage between the class of the component being rendered while still keeping the erb light and tight. It's more direct than a flyweight registry (like w/Furniture and Utilities), or the 'infer-the-constant/partial' that Rails leans into, and it gives us affordances for pulling common stuff into mixins or parent classes. I do like pulling the |
I tried adding this to def self.render(**kwargs, &block)
render(new(**kwargs), &block)
end But it doesn't work, because So if we want to do something like this, we would add it as a method to our |
DesignLibrary
#1187This is probably a premature optimization, but I hate writing the entire class name out in template files! I don't know why! It just bothers me!